apollo_network_benchmark: added broadcast topic metrics#11545
Conversation
guy-starkware
left a comment
There was a problem hiding this comment.
@guy-starkware reviewed 1 file and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @noamsp-starkware).
|
There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale. |
1f0509d to
ab1a950
Compare
602db52 to
3731f9f
Compare
ab1a950 to
2d9dcd6
Compare
2d9dcd6 to
8214b79
Compare
15e099b to
9f2229a
Compare
8214b79 to
5095a65
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| tps * message_size_bytes.into_f64() | ||
| } | ||
|
|
||
| /// Creates barebones network metrics |
There was a problem hiding this comment.
New broadcast metrics are never registered
Medium Severity
The three newly added metrics — NETWORK_STRESS_TEST_SENT_MESSAGES, NETWORK_STRESS_TEST_RECEIVED_MESSAGES, and NETWORK_DROPPED_BROADCAST_MESSAGES — are defined in the define_metrics! macro and wired into BroadcastNetworkMetrics in create_network_metrics(), but register_metrics() was not updated to call .register() on any of them. Without registration, increments to these counters won't be reported by the metrics system, defeating the purpose of this PR.
Additional Locations (1)
5095a65 to
6d10db7
Compare
9f2229a to
3e36182
Compare



Note
Low Risk
Benchmark-only change that adds new counters and wires them into
NetworkMetrics; low blast radius, with minimal risk beyond potential metrics registration/labeling mistakes.Overview
The broadcast network stress test node now emits topic-scoped broadcast metrics: counters for sent/received messages and a labeled counter for dropped messages by drop reason.
create_network_metrics()now builds aBroadcastNetworkMetricsentry keyed byTOPIC.hash()and supplies it viaNetworkMetrics.broadcast_metrics_by_topic(instead of leaving itNone), enabling the network layer to attribute metrics to the stress-test broadcast topic.Written by Cursor Bugbot for commit 6d10db7. This will update automatically on new commits. Configure here.